Skip to content

Conversation

@dklochkov-emb
Copy link
Contributor

SYCL no longer supports 2017/1.2.1 standard and it is recommended to move to 2020 standard as soon as possible. It was done:

  1. -sycl-std has only one possible value: 2020.
  2. Tests were fixed to check that -sycl-std=2017, -sycl-std=121 and -sycl-std=1.2.1 produce error

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Dec 17, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 17, 2024

@llvm/pr-subscribers-clang

Author: dklochkov-intel (dklochkov-emb)

Changes

SYCL no longer supports 2017/1.2.1 standard and it is recommended to move to 2020 standard as soon as possible. It was done:

  1. -sycl-std has only one possible value: 2020.
  2. Tests were fixed to check that -sycl-std=2017, -sycl-std=121 and -sycl-std=1.2.1 produce error

Full diff: https://github.com/llvm/llvm-project/pull/120258.diff

5 Files Affected:

  • (modified) clang/include/clang/Basic/LangOptions.h (-1)
  • (modified) clang/include/clang/Driver/Options.td (+2-2)
  • (modified) clang/lib/Frontend/InitPreprocessor.cpp (+1-3)
  • (modified) clang/test/Preprocessor/sycl-macro.cpp (-3)
  • (modified) clang/unittests/Frontend/CompilerInvocationTest.cpp (+14-13)
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 949c8f5d448bcf..6e1117da7adeb6 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -148,7 +148,6 @@ class LangOptionsBase {
 
   enum SYCLMajorVersion {
     SYCL_None,
-    SYCL_2017,
     SYCL_2020,
     // The "default" SYCL version to be used when none is specified on the
     // frontend command line.
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 88862ae9edb29d..8493ff98d23047 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8346,8 +8346,8 @@ def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group<sycl_Group>,
   Flags<[NoArgumentUnused]>,
   Visibility<[ClangOption, CC1Option, CLOption]>,
   HelpText<"SYCL language standard to compile for.">,
-  Values<"2020,2017,121,1.2.1,sycl-1.2.1">,
-  NormalizedValues<["SYCL_2020", "SYCL_2017", "SYCL_2017", "SYCL_2017", "SYCL_2017"]>,
+  Values<"2020">,
+  NormalizedValues<["SYCL_2020"]>,
   NormalizedValuesScope<"LangOptions">,
   MarshallingInfoEnum<LangOpts<"SYCLVersion">, "SYCL_None">,
   ShouldParseIf<!strconcat(fsycl_is_device.KeyPath, "||", fsycl_is_host.KeyPath)>;
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp
index e20feedb840b51..6aa71cb0ee10d5 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -584,9 +584,7 @@ static void InitializeStandardPredefinedMacros(const TargetInfo &TI,
 
   if (LangOpts.SYCLIsDevice || LangOpts.SYCLIsHost) {
     // SYCL Version is set to a value when building SYCL applications
-    if (LangOpts.getSYCLVersion() == LangOptions::SYCL_2017)
-      Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
-    else if (LangOpts.getSYCLVersion() == LangOptions::SYCL_2020)
+    if (LangOpts.getSYCLVersion() == LangOptions::SYCL_2020)
       Builder.defineMacro("SYCL_LANGUAGE_VERSION", "202012L");
   }
 
diff --git a/clang/test/Preprocessor/sycl-macro.cpp b/clang/test/Preprocessor/sycl-macro.cpp
index a509086f99e413..23228d326575b9 100644
--- a/clang/test/Preprocessor/sycl-macro.cpp
+++ b/clang/test/Preprocessor/sycl-macro.cpp
@@ -1,9 +1,6 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -sycl-std=2017 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
-// RUN: %clang_cc1 %s -fsycl-is-host -sycl-std=2017 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
 // RUN: %clang_cc1 %s -fsycl-is-host -sycl-std=2020 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD-2020 %s
 // RUN: %clang_cc1 %s -fsycl-is-host -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD-2020 %s
-// RUN: %clang_cc1 %s -fsycl-is-device -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
 // RUN: %clang_cc1 %s -fsycl-is-device -sycl-std=2020 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD-2020 %s
 // RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL %s
 
diff --git a/clang/unittests/Frontend/CompilerInvocationTest.cpp b/clang/unittests/Frontend/CompilerInvocationTest.cpp
index 94ab9fe8451e0a..dcaa6edf9b925e 100644
--- a/clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ b/clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -603,7 +603,7 @@ TEST_F(CommandLineTest, ConditionalParsingIfFalseFlagNotPresent) {
 }
 
 TEST_F(CommandLineTest, ConditionalParsingIfFalseFlagPresent) {
-  const char *Args[] = {"-sycl-std=2017"};
+  const char *Args[] = {"-sycl-std=2020"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
@@ -641,16 +641,17 @@ TEST_F(CommandLineTest, ConditionalParsingIfOddSyclStdArg1) {
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
-  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Diags->hasErrorOccurred());
   ASSERT_TRUE(Invocation.getLangOpts().SYCLIsDevice);
   ASSERT_FALSE(Invocation.getLangOpts().SYCLIsHost);
-  ASSERT_EQ(Invocation.getLangOpts().getSYCLVersion(), LangOptions::SYCL_2017);
+  ASSERT_EQ(Invocation.getLangOpts().getSYCLVersion(), LangOptions::SYCL_None);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
+  
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fsycl-is-device")));
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fsycl-is-host"))));
-  ASSERT_THAT(GeneratedArgs, Contains(HasSubstr("-sycl-std=2017")));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(HasSubstr("-sycl-std=2020"))));
 }
 
 TEST_F(CommandLineTest, ConditionalParsingIfOddSyclStdArg2) {
@@ -658,16 +659,16 @@ TEST_F(CommandLineTest, ConditionalParsingIfOddSyclStdArg2) {
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
-  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Diags->hasErrorOccurred());
   ASSERT_TRUE(Invocation.getLangOpts().SYCLIsDevice);
   ASSERT_FALSE(Invocation.getLangOpts().SYCLIsHost);
-  ASSERT_EQ(Invocation.getLangOpts().getSYCLVersion(), LangOptions::SYCL_2017);
+  ASSERT_EQ(Invocation.getLangOpts().getSYCLVersion(), LangOptions::SYCL_None);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fsycl-is-device")));
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fsycl-is-host"))));
-  ASSERT_THAT(GeneratedArgs, Contains(HasSubstr("-sycl-std=2017")));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(HasSubstr("-sycl-std=2020"))));
 }
 
 TEST_F(CommandLineTest, ConditionalParsingIfOddSyclStdArg3) {
@@ -675,16 +676,16 @@ TEST_F(CommandLineTest, ConditionalParsingIfOddSyclStdArg3) {
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
-  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Diags->hasErrorOccurred());
   ASSERT_TRUE(Invocation.getLangOpts().SYCLIsDevice);
   ASSERT_FALSE(Invocation.getLangOpts().SYCLIsHost);
-  ASSERT_EQ(Invocation.getLangOpts().getSYCLVersion(), LangOptions::SYCL_2017);
+  ASSERT_EQ(Invocation.getLangOpts().getSYCLVersion(), LangOptions::SYCL_None);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fsycl-is-device")));
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fsycl-is-host"))));
-  ASSERT_THAT(GeneratedArgs, Contains(HasSubstr("-sycl-std=2017")));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(HasSubstr("-sycl-std=2020"))));
 }
 
 TEST_F(CommandLineTest, ConditionalParsingIfTrueFlagNotPresentHost) {
@@ -718,17 +719,17 @@ TEST_F(CommandLineTest, ConditionalParsingIfTrueFlagNotPresentDevice) {
 }
 
 TEST_F(CommandLineTest, ConditionalParsingIfTrueFlagPresent) {
-  const char *Args[] = {"-fsycl-is-device", "-sycl-std=2017"};
+  const char *Args[] = {"-fsycl-is-device", "-sycl-std=2020"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
   ASSERT_FALSE(Diags->hasErrorOccurred());
-  ASSERT_EQ(Invocation.getLangOpts().getSYCLVersion(), LangOptions::SYCL_2017);
+  ASSERT_EQ(Invocation.getLangOpts().getSYCLVersion(), LangOptions::SYCL_2020);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fsycl-is-device")));
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-sycl-std=2017")));
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-sycl-std=2020")));
 }
 
 // Wide integer option.

@github-actions
Copy link

github-actions bot commented Dec 17, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@dklochkov-emb dklochkov-emb force-pushed the sycl-remove-unsupported-versions branch from 30106cd to 55996aa Compare December 17, 2024 16:31
[SYCL] fix formatting

[SYCL] remove spaces
@dklochkov-emb dklochkov-emb force-pushed the sycl-remove-unsupported-versions branch from 55996aa to 00b7797 Compare December 17, 2024 16:41
@dklochkov-emb
Copy link
Contributor Author

@abhilash1910 Hi, Please, merge if it is ok.

@philnik777
Copy link
Contributor

Closing, since this is some incredibly weak argumentation. People rely on this and we generally don't try to break them. This kind of change requires and RFC and obviously a deprecation period assuming there was good reason to drop it, which I can't see here.

@philnik777 philnik777 closed this Feb 6, 2025
@AaronBallman
Copy link
Collaborator

Closing, since this is some incredibly weak argumentation. People rely on this and we generally don't try to break them. This kind of change requires and RFC and obviously a deprecation period assuming there was good reason to drop it, which I can't see here.

Agreed -- this sort of breaking change requires discussion. Please write an RFC on Discourse and get community agreement before continuing with this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants